-
Notifications
You must be signed in to change notification settings - Fork 150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat: invoice cycle changes #1331
base: main
Are you sure you want to change the base?
Conversation
The preview deployment failed. 🔴 Last updated at: 2025-01-15 09:34:29 CET |
The preview deployment failed. 🔴 Last updated at: 2025-01-15 09:34:28 CET |
The preview deployment is in progress. 🟡 Last updated at: 2025-01-15 09:34:14 CET |
The preview deployment failed. 🔴 Last updated at: 2025-01-15 09:34:28 CET |
Updated pricing estimates
…nto poc-invoice-cycle-ref
let getEstimate = async (billingPlan, collaborators, couponId) => { | ||
try { | ||
error = ''; | ||
estimation = await sdk.forConsole.billing.estimationCreateOrganization( | ||
billingPlan, | ||
couponId == '' ? null : couponId, | ||
collaborators ?? [] | ||
); | ||
} catch (e) { | ||
// | ||
error = e.message; | ||
console.log(e); | ||
} | ||
}; | ||
|
||
let getUpdatePlanEstimate = async (organizationId, billingPlan, collaborators, couponId) => { | ||
try { | ||
error = ''; | ||
estimation = await sdk.forConsole.billing.estimationUpdatePlan( | ||
organizationId, | ||
billingPlan, | ||
couponId && couponId.length > 0 ? couponId : null, | ||
collaborators ?? [] | ||
); | ||
} catch (e) { | ||
error = e.message; | ||
console.log(e); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually use "normal functions" instead of "arrow functions" unless needed. If arrow functions are necessary in this case we can use const
instead of let
} | ||
}; | ||
|
||
$: organizationId && organizationId.length > 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to check the length of organizationId
, or are there instances where the id exists but has no length?
export let billingBudget: number; | ||
|
||
let budgetEnabled = false; | ||
var estimation: Estimation; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use let
not var
export let collaborators: string[]; | ||
export let couponId: string; | ||
export let fixedCoupon = false; | ||
export let error: string = ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove the type string
it's implicitly assigned by ''
error = ''; | ||
estimation = await sdk.forConsole.billing.estimationCreateOrganization( | ||
billingPlan, | ||
couponId == '' ? null : couponId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are using ==
instead of ===
intentionally, right?
function isOrganization(org: Organization | OrganizationError): org is Organization { | ||
return (org as Organization).$id !== undefined; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this function, if we do let's write it twice. Let's keep the code DRY 👍
<section | ||
class="card u-flex u-flex-vertical u-gap-8" | ||
style:--p-card-padding="1.5rem" | ||
style:--p-card-border-radius="var(--border-radius-small)"> | ||
<slot /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use the Card
component... also why is there a <slot />
{#if error} | ||
<Alert type="error"> | ||
<span>{error}</span> | ||
</Alert> | ||
{/if} | ||
{#if estimation} | ||
{#if estimation.unpaidInvoices?.length > 0} | ||
<Alert type="warning"> | ||
<span slot="title"> | ||
You have a unpaid invoices. Please settle them first in order to delete team. | ||
</span> | ||
</Alert> | ||
|
||
<InvoicesTable invoices={estimation.unpaidInvoices} /> | ||
{/if} | ||
{/if} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect this implementation will lead to bad layout shifts, let's double check
let getEstimate = async () => { | ||
if(isCloud) { | ||
try { | ||
error = ''; | ||
estimation = await sdk.forConsole.billing.estimationDeleteOrganization( | ||
$organization.$id | ||
); | ||
} catch (e) { | ||
error = e.message; | ||
console.log(e); | ||
} | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use function
instead of an arrow function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was unable to test this modal
What does this PR do?
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)